[#633] Unify user upsert to fix concurrent race condition#650
[#633] Unify user upsert to fix concurrent race condition#650realproject7 merged 2 commits intomainfrom
Conversation
Created shared upsertUser() helper in lib/user-upsert.ts that both register-by-wallet and onboard routes now use. Key changes: - Shared findUserByWallet() for consistent user lookup - Always update by row id when existing user is known - On 23505 conflict (concurrent insert), re-query to get row id then update by id — both routes now use identical resolution - Eliminates divergent fid-vs-primary_address conflict keys Fixes #633 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The shared upsert refactor is directionally right, but the new lookup helper regresses agent-user handling.
Findings
- [high]
findUserByWallet()only checksverified_addressesandprimary_address, so both routes now miss existing rows that are keyed byagent_wallet/agent_owner.- File:
lib/user-upsert.ts:16 - Suggestion: Reuse the full wallet lookup behavior from
getUserFromDB()(includingagent_walletandagent_owner), or otherwise extendfindUserByWallet()so agent users participate in the same cooldown and id-based upsert path.
- File:
Decision
Requesting changes because the issue explicitly requires preserving correct handling for agent users, and the new helper can return null for those rows, which breaks the conflict-retry/update path in both routes.
Aligns findUserByWallet() with the full lookup chain in getUserFromDB() so agent users are found correctly during upsert conflict resolution. Addresses T2a review feedback on PR #650. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The shared upsert refactor now preserves the broader wallet lookup behavior for agent users while unifying both routes onto the same id-based conflict resolution path.
Findings
- None.
Decision
Approving on code review. The earlier agent-user regression is addressed; the remaining CI checks are still running, but they do not block the review verdict.
2 participants
Summary
lib/user-upsert.ts: SharedfindUserByWallet()andupsertUser()helpers with consistent conflict resolution — always updates by rowid, re-queries on 23505 conflict to find the correct row.Previously, concurrent calls used different conflict keys (
fidvsprimary_address) which could corrupt rows with mixed data.Fixes #633
Tracks realproject7/agent-os#313
Test plan
🤖 Generated with Claude Code